tests/pkg/emlearn: add model.h to repo #20841
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maribu
requested review from
leandrolanzieri,
aabadie and
MichelRottleuthner
as code owners
mguetschow
approved these changes
Aug 28, 2024
There was a problem hiding this comment.
Sounds good to me to have model.h checked in and updated in lockstep with the binary model for new versions of emlearn as this relieves the CI from having to contain emlearn.
CI complains about trailing whitespaces and non-C++ compatible header. Either we would have to add some sed magic to resolve those on model.h (likely should be part of the Make target) or we exclude that specific header from the static checks, if that is somehow possible.
mguetschow
added
the
CI: ready for build
mguetschow
added
CI: ready for build
|
After rebasing to master, this should be ready to be merged @maribu. |
The Makefile rule to regenerate model.h has been left in place, but just adding the header to the test allows to build the application without heaving emlearn installed, which is convenient for the CI. Actual users of emlearn will like still want to generate the header.
maribu
force-pushed
the
tests/pkg/emlearn
branch
from
7552343 to
2c89b7a
Compare
Thanks for the reminder. Done :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contribution description
This adds the previously on-demand generated
model.hto the repo. The Makefile rule to regenerate model.h has been left in place, but just adding the header to the test allows to build the application without heaving emlearn installed, which is convenient for the CI.Actual users of emlearn will likely still want to generate the header.
Testing procedure
make -C tests/pkg/emlearnshould now work despiteemlearnnot installed.Issues/PRs references
This should fix the issue from #20840